Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

runtime: remove randomness collective flip pallet#3347

Merged
2 commits merged intomasterfrom
andre/remove-randomness-collective-flip
Jun 22, 2021
Merged

runtime: remove randomness collective flip pallet#3347
2 commits merged intomasterfrom
andre/remove-randomness-collective-flip

Conversation

@andresilva
Copy link
Copy Markdown
Contributor

@andresilva andresilva commented Jun 22, 2021 •
edited
Loading

This is insecure and should not be used, in particular as we have BABE as a better source of randomness.

@andresilva andresilva added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 22, 2021
@andresilva
Copy link
Copy Markdown
Contributor Author

andresilva commented Jun 22, 2021 •
edited
Loading

This pallet stored the last 81 block hashes in its storage, should we write a runtime migration to clean that up?

@andresilva andresilva requested a review from shawntabrizi June 22, 2021 21:57
@ordian ordian added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Jun 22, 2021
ordian
ordian reviewed Jun 22, 2021
View reviewed changes
@shawntabrizi
Copy link
Copy Markdown
Member

shawntabrizi commented Jun 22, 2021 •
edited
Loading

This pallet stored the last 81 block hashes in its storage, should we write a runtime migration to clean that up?

Yes, probably. Or we can do governance motion.

rphmeier
rphmeier approved these changes Jun 22, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bless up

@shawntabrizi
add storage migration to remove key
29bde59 
Co-Authored-By: parity-processbot <>
@shawntabrizi
Copy link
Copy Markdown
Member

shawntabrizi commented Jun 22, 2021 •
edited
Loading

@andresilva take a look at the migration. if all looks okay, you can merge

we should take a look at this migration on westend when that happens

@andresilva
Copy link
Copy Markdown
Contributor Author

andresilva commented Jun 22, 2021

bot merge

@ghost
Copy link
Copy Markdown

ghost commented Jun 22, 2021

Trying merge.

@ghost ghost merged commit 869a130 into master Jun 22, 2021
@ghost ghost deleted the andre/remove-randomness-collective-flip branch June 22, 2021 23:36
@s3krit s3krit mentioned this pull request Jun 30, 2021
Merged
@apopiak apopiak mentioned this pull request Oct 19, 2021
Closed
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@ordian ordian ordian left review comments

@rphmeier rphmeier rphmeier approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andresilva @shawntabrizi @ordian @rphmeier